Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle np.array(DataIO) for numpy in py38 #248

Merged
merged 5 commits into from
Jan 9, 2020
Merged

Handle np.array(DataIO) for numpy in py38 #248

merged 5 commits into from
Jan 9, 2020

Conversation

rly
Copy link
Contributor

@rly rly commented Jan 9, 2020

Partially address #245

@rly rly requested a review from a team January 9, 2020 02:15
@rly rly mentioned this pull request Jan 9, 2020
@codecov
Copy link

codecov bot commented Jan 9, 2020

Codecov Report

❗ No coverage uploaded for pull request base (dev@7831820). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##             dev    #248   +/-   ##
=====================================
  Coverage       ?   70.3%           
=====================================
  Files          ?      33           
  Lines          ?    5938           
  Branches       ?    1394           
=====================================
  Hits           ?    4175           
  Misses         ?    1328           
  Partials       ?     435

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7831820...79302d8. Read the comment docs.

@@ -626,6 +626,8 @@ def __bool__(self):

def __getattr__(self, attr):
"""Delegate attribute lookup to data object"""
if attr == '__array_struct__': # np.array() checks __array__ or __array_struct__ attribute dep. on version
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean delegation of __array__ is allows but delegation of __array_struct__ is not? I.e., depending on the version of numpy we allow conversion of DataIO to a numpy array or not? Could you clarify what the problem with the __array_struct__ method is?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake. The error should only be thrown if the object is not self.valid. I have added that back.

oruebel
oruebel previously approved these changes Jan 9, 2020
@rly
Copy link
Contributor Author

rly commented Jan 9, 2020

To clarify: np.array() with python 3.8 was calling self.getattr('__array_struct__') or possibly self.hasattr('__array_struct__'). I could not override the method __array_struct__ with the below:

def __array_struct__(self):
        if not self.valid:
            raise InvalidDataIOError("Cannot convert data to array. Data is not valid.")
        return self.__array_struct__()

I would get the error: 'ValueError: invalid __array_struct__' on calls to np.array() that worked otherwise. Maybe there is an issue with name mangling. So instead of overriding the __array_struct__ method, I check for the attribute argument __array_struct__ in the getattr method.

@rly rly merged commit 5e08814 into dev Jan 9, 2020
@rly rly deleted the fix/numpy_py38 branch January 9, 2020 20:31
@rly rly mentioned this pull request Jan 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants